Skip to content

Per 10621 thumbnail autorefresh - #1113

Open
cecilia-donnelly wants to merge 4 commits into
mainfrom
per-10621-thumbnail-autorefresh
Open

Per 10621 thumbnail autorefresh#1113
cecilia-donnelly wants to merge 4 commits into
mainfrom
per-10621-thumbnail-autorefresh

Conversation

@cecilia-donnelly

@cecilia-donnelly cecilia-donnelly commented Aug 2, 2026 •
edited by aasandei-vsp
Loading

Copy link
Copy Markdown
Member

We had a couple bugs preventing thumbnails from appearing immediately on upload, as they should. This is adding some perceived slowness to the app.

These changes need careful review, but seem to make thumbnails appear on upload again which is desirable.

Warning

The major issue here is that the fixes need to touch a service DataService and a component FolderPickerComponent that are present in a lot of places and the code we introduce cannot be isolated. The fixes will need a lot of testing outside the thumbnails scope.
Also, this fix just bypasses a big problem we are having accross the app. We are mutating objects in place instead of replacing them, so even if the reference is the same, the object has changed. This makes any object highly unreliable for using it inside an Angular context.

This would be the behavior for records:
https://www.loom.com/share/b3cbb52bf5bc4c03a0bf2c57d658ad83

Manual test cases

Thumbnail appears after upload

Single image

  1. Upload one JPG into the open folder.
    • EXPECTED: the row appears immediately with no thumbnail, then the thumbnail fills in on its own within a few seconds. No page refresh, no navigating away.

Several images at once

  1. Upload 5 JPGs in one go. Do not do anything else.(click, reload etc.).
    • EXPECTED: every one of them gets its thumbnail, not all at once. None is left permanently blank.
  2. Repeat with a mix of file types (JPG, PNG, PDF, a .zip).
    • EXPECTED: images and PDFs get thumbnails; the zip keeps its archive icon; nothing else is disturbed.

Leaving the folder mid-refresh

  1. Upload a JPG and immediately navigate to another folder before the thumbnail appears.
    • EXPECTED: navigation is instant and the new folder behaves normally.
  2. Navigate back into the original folder.
    • EXPECTED: the thumbnail is there or appears shortly.

Share previews

(covered by specs — still verify once by hand, this is the highest-risk behaviour in the branch)

Listed share (preview toggle on)

  1. Open a share preview link for a share that is restricted.
    • EXPECTED: every record shows a stock placeholder image from assets/img/preview/. No real file content is visible anywhere on the page.
  2. Wait 10 seconds on the page, then look again.
    • EXPECTED: still stock images. A late-arriving real thumbnail must never replace one.

Unlisted share

  1. Open an unlisted share link.
    • EXPECTED: real thumbnails are shown.

Public archive

Record thumbnails on first load

  1. Open a public archive that contains images.
    • EXPECTED: grid view, and every image tile shows its thumbnail without a refresh.

Folder tiles

  1. Look at a folder tile whose folder contains images.
    • EXPECTED: the tile shows a thumbnail borrowed from one of those images.
  2. Open a folder that is empty.
    • EXPECTED: the tile shows the folder_open icon rather than a broken image.
  3. Open a folder containing only subfolders, and one containing only non-image files.
    • EXPECTED: perm_media for subfolders-only, description for mixed non-image files. No blank tiles.

Choosing a profile photo or banner

Thumbnails on first open

  1. Open the profile panel from the left menu, click the profile photo, and navigate into a folder of images.
    • EXPECTED: thumbnails appear as soon as the folder lists. You should not have to select an image or leave and come back to make them show up.
  2. Do the same for Change Banner.
    • EXPECTED: same behaviour.

Selected record preview in picker

  1. While in the profile panel, image profile picker, click one of the images in the picker.
    • EXPECTED: the large preview above the footer shows that image, not a blank box.
  2. Press Back.
    • EXPECTED: you return to the list with thumbnails still showing.

The photo updates in place

  1. While in the profile panel, image profile picker, choose an image and confirm.
    • EXPECTED: the profile photo in the panel updates immediately after the picker closes. No refresh, no navigating away.
  2. Change the banner too.
    • EXPECTED: the banner updates immediately, as it did before this branch.

Cancelling

  1. Open the picker for the profile photo and cancel without choosing.
    • EXPECTED: the existing photo is unchanged.
  2. Do the same for the banner.
    • EXPECTED: the existing banner is unchanged.

@codecov

codecov Bot commented Aug 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.12195% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.36%. Comparing base (6103d93) to head (97ee045).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
.../components/profile-edit/profile-edit.component.ts 33.33% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1113      +/-   ##
==========================================
+ Coverage   52.33%   52.36%   +0.02%     
==========================================
  Files         354      354              
  Lines       12093    12117      +24     
  Branches     2186     2191       +5     
==========================================
+ Hits         6329     6345      +16     
- Misses       5538     5549      +11     
+ Partials      226      223       -3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cecilia-donnelly
cecilia-donnelly force-pushed the per-10621-thumbnail-autorefresh branch 4 times, most recently from d7506a6 to 3249b4a Compare August 2, 2026 13:38
@cecilia-donnelly

cecilia-donnelly commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

@aasandei-vsp , can we pair on this? It is a bigger change than I expected when I picked it up! I thought the first commit would do it.

@cecilia-donnelly

cecilia-donnelly commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

This appears to be working, but a note from Claude for our discussion: "Two threads for later: the id normalization to strings (which retires isSameId), and hideItemsInCurrentFolder carrying the same parentFolder_linkId mismatch — likely swept up by the same work."

@joneran1982 joneran1982 mentioned this pull request Aug 3, 2026
Closed
@aasandei-vsp
aasandei-vsp force-pushed the per-10621-thumbnail-autorefresh branch from 3249b4a to 74a5094 Compare August 4, 2026 11:50
@aasandei-vsp aasandei-vsp self-assigned this Aug 5, 2026
@aasandei-vsp
aasandei-vsp requested review from slifty and removed request for aasandei-vsp August 5, 2026 14:57
@aasandei-vsp
aasandei-vsp force-pushed the per-10621-thumbnail-autorefresh branch from 6b23183 to 51aa47b Compare August 5, 2026 14:58
@aasandei-vsp
aasandei-vsp marked this pull request as ready for review August 5, 2026 14:58
cecilia-donnelly
cecilia-donnelly commented Aug 5, 2026
View reviewed changes

@cecilia-donnelly cecilia-donnelly left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went through the manual tests except the profile picture ones (will do those later today). They are working! I was misled by the folder-with-only-subfolders because of some recurrence (possibly?) of the scroll bug -- I had to scroll up to the top manually. Without doing that the screen looked blank. Feel free to use my dev archive to poke around with different cases, @aasandei-vsp , since you found that public behaves differently on local: https://app.dev.permanent.org/p/archive/06ms-0000

cecilia-donnelly and others added 4 commits August 6, 2026 13:19
@cecilia-donnelly @aasandei-vsp
PER-10580: First thumbnail appears without refresh
d769b05 
This only works for the first thumbnail of an uploaded set.
@cecilia-donnelly @aasandei-vsp
PER-10580: Multiple thumbnails appear without refresh
987208e 
This is a more complex fix that should get in-depth review, since it
changes the data service and I am not confident in it. Claude suggests
that in the switch to the stela "children" endpoint we missed a couple
points. We weren't correctly tracking the responses because of the
string/number id mismatch, so there are some workarounds for that in
here.
@aasandei-vsp
Add a subscription for populating the thumbnail after it is retrieved…
f38beaf 
… from BE

After upload, the thumbnail is not immediatelly generated, so we keep calling
the BE until it is available. So the list item will subscribe to this refresh
and populate the thumbnail whenever is available.

One important thing to mention is that thumbnails should not be available for
restricted shares, so making sure that we only show it after we check if the
share is restricted or not is vital, that's why the isUnlistedShare variable
is needed.

Issue: PER-10580
@aasandei-vsp
Update thumbnails in folder picker and profile edit when they are ava…
97ee045 
…ilable

This fix just bypasses a big problem we are having accross the app. We are
mutating objects in place instead of replacing them, so even if the reference
is the same, the object has changed. This makes any object highly unreliable
for using it inside an Angular context.

In this situation, we were using a pipe for rendering the thumbnails,
which is the correct approach. The issue is the pipe would update when
the object reference changes, which never happens in our case, even
though the thumbnailUrls do.

Issue: PER-10580
@aasandei-vsp
aasandei-vsp force-pushed the per-10621-thumbnail-autorefresh branch from 51aa47b to 97ee045 Compare August 6, 2026 10:21
@aasandei-vsp

aasandei-vsp commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

I went through the manual tests except the profile picture ones (will do those later today). They are working! I was misled by the folder-with-only-subfolders because of some recurrence (possibly?) of the scroll bug -- I had to scroll up to the top manually. Without doing that the screen looked blank. Feel free to use my dev archive to poke around with different cases, @aasandei-vsp , since you found that public behaves differently on local: https://app.dev.permanent.org/p/archive/06ms-0000

Thank you for the archive! I have managed to test that part too and it all works!
So it's all green for me, just waiting for the review and then we can move to QA!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@slifty slifty Awaiting requested review from slifty

At least 1 approving review is required to merge this pull request.

Assignees

@aasandei-vsp aasandei-vsp

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cecilia-donnelly @aasandei-vsp